Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flow migrate state command #1426

Merged
merged 15 commits into from
Mar 12, 2024

Conversation

jribbink
Copy link
Contributor

@jribbink jribbink commented Feb 22, 2024

Closes #1433

Description


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@jribbink jribbink changed the title Jribbink/migrate command Add flow migrate state command to migrate emulator state to Cadence 1.0 Feb 22, 2024
@jribbink jribbink changed the title Add flow migrate state command to migrate emulator state to Cadence 1.0 Add flow migrate state command to migrate emulator state to Cadence 1.0 Feb 22, 2024
@jribbink jribbink changed the title Add flow migrate state command to migrate emulator state to Cadence 1.0 Add flow migrate state command Feb 22, 2024
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

return nil, fmt.Errorf("failed to open database: %w", err)
}

rwf := &migration.NOOPReportWriterFactory{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably want some kind of writer here to provide feedback to the developer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I'll see how I can surface it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a --save-report flag - open to feedback here about naming/acceptable defaults. Assumed it's probably best not to pollute user's workspace if not manually specified (so default to NOOP) but can change if wanted.

https://github.com/onflow/flow-cli/pull/1426/files#diff-fbaffef2f65ec644d807fb85832bf2077233decf77b14155006c99f7159cd850R43

internal/migrate/state.go Outdated Show resolved Hide resolved
internal/migrate/state.go Outdated Show resolved Hide resolved
@turbolent turbolent added the Feature A new user feature or a new package API label Feb 27, 2024
@turbolent turbolent marked this pull request as ready for review February 27, 2024 01:21
@jribbink jribbink changed the base branch from feature/stable-cadence to master February 27, 2024 09:01
@jribbink jribbink changed the base branch from master to feature/stable-cadence February 27, 2024 09:01
Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with a local build, looks good! 👌

@turbolent
Copy link
Member

Should be good to go once #1450 is in

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@turbolent
Copy link
Member

@SupunS Not quite sure about 5b2daca.

In a previous Emulator state, the Burner contract is not deployed yet, so I think deploying it is correct (instead of updating it).

What about the EVM contract though?

@SupunS
Copy link
Member

SupunS commented Mar 12, 2024

Think that should be OK. We can have the same params as in https://github.com/onflow/flow-emulator/pull/608/files#diff-717939af182d8688c65dd7e4a5b5471360a4e3719f2dba0b34629167f3ce70afR97-R104.

@turbolent turbolent merged commit e5710cb into feature/stable-cadence Mar 12, 2024
5 checks passed
@turbolent turbolent deleted the jribbink/migrate-command branch March 12, 2024 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new user feature or a new package API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants